Support iterators on GFK fields when using _quantity param #207
Conversation
There was a problem hiding this comment.
Thanks for this fix @abbottc! The only thing I'll have to ask is for you to also update the changelog file. Just copying and pasting the PR title and PR id to it is enough.
I'll wait for an extra review from @amureki @anapaulagomes or @timjklein36 to get this merged in. Once this PR and #206 are merged, we'll release a new version of the lib.
model_bakery/baker.py
Outdated
| return ( | ||
| self.model._meta.fields | ||
| + self.model._meta.many_to_many | ||
| + tuple(self.model._meta.private_fields) | ||
| ) |
There was a problem hiding this comment.
@berinhard Any reason we can't use self.model._meta.get_fields() here and then remove related_objects via set subtraction (as they are retrieved via another method already, get_related)?
| return ( | |
| self.model._meta.fields | |
| + self.model._meta.many_to_many | |
| + tuple(self.model._meta.private_fields) | |
| ) | |
| return set(self.model._meta.get_fields()) - set(self.get_related()) |
The results may be more complex to read, but it would be future-proof going forward.
There was a problem hiding this comment.
Yes, I think using Django's public API in _meta over private API is preferable too. The get_related method itself could also be updated to only use public API methods (perhaps also other areas in the Baker class), but that seems outside the scope of this PR at least. I've updated this method according to the suggestion.
| objects = baker.make(models.Profile, _quantity=2) | ||
| dummies = baker.make( | ||
| models.DummyGenericForeignKeyModel, | ||
| content_object=iter(objects), |
There was a problem hiding this comment.
What happens when no value is provided for this field? Consider adding a test for it.
Also, what happens to the content_type and object_id fields? Maybe there should be a test for those as well?
There was a problem hiding this comment.
This test might be related to the latter: https://github.com/model-bakers/model_bakery/blob/main/tests/test_filling_fields.py#L272.
There was a problem hiding this comment.
I've updated the test to also verify the content_type and object_id field contain the expected values.
I'm unclear on whether the question "What happens when no value is provided for this field?" means what happens for the case baker.make(models.DummyGenericForeignKeyModel) or the case baker.make(models.DummyGenericForeignKeyModel, content_object=iter([])), since this PR specifically fixes iter being broken on a GFK. Did you want a test for the latter? The former case is already present in an existing test.
There was a problem hiding this comment.
No, I think you are right: the case I was thinking of is already covered. No action needed.
|
Thank you for your contribution @abbottc! 🎉 |
Fixes #166